You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds a local review application on top of the plan/linking library and SQLite store. npm run demo starts a loopback-only screen backed by a real Git fixture; an existing trusted store can be opened with npm start -- --config review.json.
The screen groups changes by plan item, shows attribution/scope/tests/AI-review status, renders provenance and file metadata, persists approvals and per-item discussion, supports assignment/acceptance and no-change confirmation, and shows stale approval evidence. Refresh observes HEAD changes. Browser actions are bound to the reviewed state and an atomic review counter, so concurrent assignments cannot silently be approved from an old view. Store schema v2 transactionally migrates v1 data.
Uses DESIGN.md tokens and self-hosted Plex fonts, keyboard navigation, desktop breakpoints, and error/empty states. The HTTP boundary is loopback-only with a private token, origin/host validation, bounded UTF-8 JSON, CSP, and escaped untrusted text. No merge, agent execution, or test execution controls are included. Questions are saved rather than falsely presented as AI-answered; unavailable check evidence is labeled Not run.
Also adds a disposable-clone planting helper with ledger-aware SHA mappings and a committed experiment protocol. The source repository is unchanged by planting. Real issue pairs, the manual assignment, human timed reviews, and the final go/no-go result are pending. Raster previews and byte sizes are included; unsupported/oversized previews are labeled unavailable. This advances #3 but does not close it or claim the go/no-go gate passed.
Validation: 174 unit/integration tests (baseline 157), 13 Playwright tests, typecheck, and diff formatting pass. Chromium coverage includes persistent approvals/notes, assignment staleness, metadata, no-change confirmation, keyboard/breakpoints, stale-view rejection, origin/auth checks, history errors, untrusted text, large-change assignments, and decoded image previews. Visually inspected the desktop screenshot. CI now runs the browser suite.
Decisions, local setup, remaining work: docs/implementation/read-only-review.md. Experiment protocol: docs/experiments/review-protocol.md.
Boundary fixes before final review: a 20 KB assignment reproduced HTTP 413, fixed with opaque segment IDs while preserving stored content/copy keys. Raster previews are limited to 1 MiB per blob, 4 MiB raw across history, and 6 MiB encoded per response. Non-ASCII paths on case-insensitive filesystems fail explicitly until a filesystem-specific identity adapter is supplied.
Review round 1: reproduced and fixed no-change approval with unresolved ambiguity, malformed credential status handling, and stale controls after errors. Added standalone-acceptance persistence and whole-plan empty-state coverage. Browser assets are authored source (formatted and marked as such), not generated output. No findings declined.
Review round 2: reproduced mixed owned/ambiguous approval succeeding, then fixed the runner guard and UI routing for every ambiguous item. The guard now applies regardless of owned-change count. No findings declined.
Review round 3: reproduced and fixed demo config/symlink escapes and inherited Git environment redirection. Demo and planting commands now share a case-insensitive Git environment scrub; four helper regressions exercise these boundaries. Also reproduced and fixed keyboard shortcuts becoming inactive when toolbar buttons have focus. No findings declined.
Review round 4: no inline findings; addressed both summary concerns with reproduced regressions. Planting applies the viewer’s non-ASCII case-insensitive path guard, and accepted cards explicitly say “Accepted outside plan” while retaining provenance. No concerns declined.
Review round 5: reproduced and fixed symlinked demo ancestors and canonical filename collisions. Planting checks declared paths plus base/commit tree paths through the configured identity before cloning. No actionable findings declined; the summary also mentioned status styling without a concrete example (accepted-row labeling was fixed in round 4).
Review round 6: fixed a reproduced destination-inside-source boundary violation, including canonical symlink aliases. Declined the Git-environment claim: the wrapper already passes env, and a new end-to-end inherited-variable test passed before production changes. The unspecified snapshot-race summary is guarded by view consistency checks and atomic write CAS with concurrent-write tests.
Review round 7: fixed reproduced oversized image dimensions and dangling SQLite sidecar links. Preview metadata must meet per-side (8192), per-image (4M pixels) and cumulative unique-blob (16M pixels) limits as well as byte limits. Unknown dimensions omit the preview. Both actionable findings addressed.
Final review status: eight automated review rounds; the final round on e582882 reported no new findings. All ten concrete review threads are resolved. One environment-passing false positive was declined with passing end-to-end evidence. Summary-only rename/literal-path/styling concerns are preserved for targeted validation in #10. Both CI runs, typecheck, 174 unit/integration tests and 13 browser tests pass. User authorized merging. Issue #3 remains open for the real-issue experiment.
Adds a loopback-only, plan-indexed review application backed by Git and SQLite, with persistent review state, browser coverage, and experiment tooling.
Changes:
Added review server, CLI, responsive UI, authentication, and persistence.
Added approvals, notes, assignments, stale-state handling, and schema migration.
Added demo/planting tools, Playwright tests, CI integration, and documentation.
File
Summary
web/server.ts
Loopback HTTP server and authenticated API boundary
Round 1 response: fixed the no-change/ambiguity bypass, malformed-token handling, and stale/error control state with failing-before/passing-after regressions. Added standalone acceptance persistence and whole-plan empty-state browser coverage. No findings declined.
The update also completes bounded raster previews/byte sizes and fixes large-change assignment commands via opaque IDs (20 KB regression first returned HTTP 413). The hand-authored JS/CSS/HTML are formatted and explicitly marked as authored source; please include them in review rather than treating them as generated files.
Validation on 19aabad: 163 unit/integration tests, 11 browser tests, typecheck, and diff checks pass. Requesting a fresh review.
Compare canonical paths for case-insensitive collision checks
scripts/plant.ts:28
This collision check compares raw spellings even though pathKey is explicitly configured for case-insensitive filesystems. With a declared retry.ts and an input path RETRY.TS, planting proceeds and the viewer canonicalizes both paths to the same identity, so the supposedly undeclared plant is classified in-scope. Compare canonical pathKey values before creating the clone.
Detect existing paths using configured case-insensitive identity
scripts/plant.ts:46
The existing-path guard also uses an exact Git pathspec, so on a case-sensitive source checkout configured with caseSensitive: false, an existing FOO.TXT is not detected when the requested plant is foo.txt. The clone can then create an identity alias that the linking layer treats as the same path, invalidating the experiment's out-of-scope classification. Enumerate the relevant tree paths and compare them through the configured identity function (including the base tree), rather than checking only the exact spelling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a local review application on top of the plan/linking library and SQLite store.
npm run demostarts a loopback-only screen backed by a real Git fixture; an existing trusted store can be opened withnpm start -- --config review.json.The screen groups changes by plan item, shows attribution/scope/tests/AI-review status, renders provenance and file metadata, persists approvals and per-item discussion, supports assignment/acceptance and no-change confirmation, and shows stale approval evidence. Refresh observes HEAD changes. Browser actions are bound to the reviewed state and an atomic review counter, so concurrent assignments cannot silently be approved from an old view. Store schema v2 transactionally migrates v1 data.
Uses DESIGN.md tokens and self-hosted Plex fonts, keyboard navigation, desktop breakpoints, and error/empty states. The HTTP boundary is loopback-only with a private token, origin/host validation, bounded UTF-8 JSON, CSP, and escaped untrusted text. No merge, agent execution, or test execution controls are included. Questions are saved rather than falsely presented as AI-answered; unavailable check evidence is labeled Not run.
Also adds a disposable-clone planting helper with ledger-aware SHA mappings and a committed experiment protocol. The source repository is unchanged by planting. Real issue pairs, the manual assignment, human timed reviews, and the final go/no-go result are pending. Raster previews and byte sizes are included; unsupported/oversized previews are labeled unavailable. This advances #3 but does not close it or claim the go/no-go gate passed.
Validation: 174 unit/integration tests (baseline 157), 13 Playwright tests, typecheck, and diff formatting pass. Chromium coverage includes persistent approvals/notes, assignment staleness, metadata, no-change confirmation, keyboard/breakpoints, stale-view rejection, origin/auth checks, history errors, untrusted text, large-change assignments, and decoded image previews. Visually inspected the desktop screenshot. CI now runs the browser suite.
Decisions, local setup, remaining work:
docs/implementation/read-only-review.md. Experiment protocol:docs/experiments/review-protocol.md.Boundary fixes before final review: a 20 KB assignment reproduced HTTP 413, fixed with opaque segment IDs while preserving stored content/copy keys. Raster previews are limited to 1 MiB per blob, 4 MiB raw across history, and 6 MiB encoded per response. Non-ASCII paths on case-insensitive filesystems fail explicitly until a filesystem-specific identity adapter is supplied.
Review round 1: reproduced and fixed no-change approval with unresolved ambiguity, malformed credential status handling, and stale controls after errors. Added standalone-acceptance persistence and whole-plan empty-state coverage. Browser assets are authored source (formatted and marked as such), not generated output. No findings declined.
Review round 2: reproduced mixed owned/ambiguous approval succeeding, then fixed the runner guard and UI routing for every ambiguous item. The guard now applies regardless of owned-change count. No findings declined.
Review round 3: reproduced and fixed demo config/symlink escapes and inherited Git environment redirection. Demo and planting commands now share a case-insensitive Git environment scrub; four helper regressions exercise these boundaries. Also reproduced and fixed keyboard shortcuts becoming inactive when toolbar buttons have focus. No findings declined.
Review round 4: no inline findings; addressed both summary concerns with reproduced regressions. Planting applies the viewer’s non-ASCII case-insensitive path guard, and accepted cards explicitly say “Accepted outside plan” while retaining provenance. No concerns declined.
Review round 5: reproduced and fixed symlinked demo ancestors and canonical filename collisions. Planting checks declared paths plus base/commit tree paths through the configured identity before cloning. No actionable findings declined; the summary also mentioned status styling without a concrete example (accepted-row labeling was fixed in round 4).
Review round 6: fixed a reproduced destination-inside-source boundary violation, including canonical symlink aliases. Declined the Git-environment claim: the wrapper already passes
env, and a new end-to-end inherited-variable test passed before production changes. The unspecified snapshot-race summary is guarded by view consistency checks and atomic write CAS with concurrent-write tests.Review round 7: fixed reproduced oversized image dimensions and dangling SQLite sidecar links. Preview metadata must meet per-side (8192), per-image (4M pixels) and cumulative unique-blob (16M pixels) limits as well as byte limits. Unknown dimensions omit the preview. Both actionable findings addressed.
Final review status: eight automated review rounds; the final round on e582882 reported no new findings. All ten concrete review threads are resolved. One environment-passing false positive was declined with passing end-to-end evidence. Summary-only rename/literal-path/styling concerns are preserved for targeted validation in #10. Both CI runs, typecheck, 174 unit/integration tests and 13 browser tests pass. User authorized merging. Issue #3 remains open for the real-issue experiment.